Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed workaround for old Webkit bug in the TinyMCE editor for selec… #9540

Conversation

hostep
Copy link
Contributor

@hostep hostep commented May 7, 2017

…tions of images and hr elements, because this workaround now causes problems in Google Chrome version 58.

Description

This PR attempts to solve a new bug which popped up by upgrading Google Chrome to version 58.
In the TinyMCE editor, we can no longer select images (or widgets, which are represented by an image), so we can no longer edit images (or widgets) using the TinyMCE editor.
See #9518 for more detailed info with links to a bunch of external sources.

Fixed Issues (if relevant)

  1. Chrome version 58 causes problems with selections in the tinymce editor #9518: Chrome version 58 causes problems with selections in the TinyMCE editor

Manual testing scenarios

  1. Start editing a CMS page in the backend of Magento using Google Chrome version 58
  2. Make sure you have a CMS page which contains an image and/or a widget
  3. Try to select the image or the widget, by clicking it
  4. You won't succeed, because a JS error is being thrown: Failed to execute 'setBaseAndExtent' on 'Selection': There is no child at offset 1.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

Discussion

I only briefly tested these changes, only on Chrome 58 and Safari 10.1, so it would be highly appreciated if someone else can test all browsers which Magento supports and see if this doesn't break any other browser.

I applied the changes to all tiny_mce scripts I could find, although I think only the tiny_mce_src.js one is used in the backend of Magento. But since you bundle all the other scripts in Magento, It could be possible that someone is using another script, so I assumed this should be fixed in all the tiny_mce scripts.
Also: the minified tiny_mce scripts were manually edited, I don't know the command which was being used to generate the minified scripts, so please double check that my manual edits are ok.

Since this bug will probably popup with a lot of shopowners in the near future, this fix should get a bit of a higher priority and should also get backported to Magento versions 2.1 and 2.0 as soon as possible.
If this PR is accepted, I can create a backport for version 2.1, but not sure how backporting to 2.0 is supposed to work?

…tions of images and hr elements, because this workaround now causes problems in Google Chrome version 58.
@hostep
Copy link
Contributor Author

hostep commented May 10, 2017

I just noticed this problem is also manifested in Magento 1 shops in Chrome 58. So if Magento releases a new version for version 1, this should best be included as well I believe (maybe even in a SUPEE patch, so we can apply it to older Magento versions as well?).

@okorshenko: any reasons why this PR is being ignored, or is it still under investigation by Magento?
I believe it's a pretty high priority problem, since the Google Chrome browser is now the dominant player and most people will already have upgraded to version 58 and will no longer be able to use the TinyMCE editor in the backend of Magento...

Thanks!

@ishakhsuvarov ishakhsuvarov self-assigned this May 13, 2017
@ishakhsuvarov ishakhsuvarov added this to the May 2017 milestone May 13, 2017
@magento-team magento-team merged commit 4d10f05 into magento:develop May 16, 2017
magento-team pushed a commit that referenced this pull request May 16, 2017
@magento-team
Copy link
Contributor

@hostep thank you for your contribution. Your Pull Request has been successfully merged

magento-team pushed a commit that referenced this pull request Jun 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants